Position editor lines by their real heights when they vary by webergnr · Pull Request #1084 · lapce/floem

webergnr · 2026-08-21T20:37:45Z

Position editor lines by their real heights when they vary

The bug

normal_compute_screen_lines places every visual line at y_idx * line_height,
where line_height is that line's height — so the grid step comes from
whichever line is being placed. The code says as much:

// TODO: don't assume universal line height!
let line_height = style.line_height(editor.id(), 0);

With a uniform styling this is correct and O(1). As soon as Styling::line_height
varies per line — a markdown editor sizing headings, a diff view with a taller
row — lines overlap: a 31px heading followed by a 20px body line puts the body at
1 * 20 = 20, eleven pixels inside the heading. line_col_of_point, the document
height and ensure_visible divide or multiply by the same single height, so
hit-testing and caret-following drift with it.

The fix

A Styling::uniform_line_height(edid) -> bool hint, defaulting to true:
every existing styling keeps the arithmetic path exactly as it is today, and
nothing gets slower. A styling that returns false gets positions accumulated
from the real heights:

Editor::line_y / rvline_y / rvline_at_y / line_height_total /
total_height walk the heights.
Wrapped lines are included: a line's height is line_height × rows, and the
row count is read off the cached text layout via Lines::cached_line_count —
a line with no layout yet counts as one, the same assumption
Lines::last_vline already makes, so no layout is forced and the accuracy
model is unchanged.
normal_compute_screen_lines starts from the visual line containing y0 and
stops at the first row past y1 instead of counting a vline span.
The same walk backs line_col_of_point(_with_phantom), EditorView's and the
gutter's content height, and the ensure_visible rect.

The cost when the hint is false is a walk over the lines above the viewport,
the same order of work Lines::last_vline already does when wrapping is on.

Tests

row_at_y — the walk itself — is a free function with four tests: a uniform
document lands where division would, a tall line pushes the ones under it down,
a wrapped line owns a row per wrap, and past the last line there is no row. The
existing suite (107) stays green.

Where it is used

A markdown notes editor whose buffer holds headings and body text in one
text_editor, at the sizes their own styling gives them: before this, the body
line under every heading was drawn inside it. Happy to add before/after
screenshots, a Styling example, or a changelog entry if you'd like them here.

…pped normal_compute_screen_lines placed every visual line at index * its own line height, which is only correct when all lines share one height (the in-code TODO acknowledged this). With WrapMethod::None every buffer line is one visual line, so real heights can be summed without forcing text layouts: screen line positions, point<->line hit testing and the total document height now accumulate style.line_height per line. Wrapped editors keep the uniform-grid behaviour unchanged.

Copilot

Pull request overview

Adds opt-in variable-height editor positioning while preserving the uniform-height fast path.

Changes:

Adds uniform_line_height support and cached wrapped-row counting.
Updates rendering, hit-testing, caret visibility, and content sizing.
Adds tests and changelog documentation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Summary
`src/views/editor/visual_line.rs`	Adds cached visual-line counts.
`src/views/editor/view.rs`	Updates sizing and caret visibility. Moderate issue: bottom-scroll margin uses the first line’s height instead of the last line’s height.
`src/views/editor/text.rs`	Adds the styling capability hint.
`src/views/editor/mod.rs`	Implements variable-height positioning and hit-testing. Moderate issue: lazy layout creation can offset positions when the created line has a different height.
`src/views/editor/gutter.rs`	Uses accumulated content height. Moderate issue: bottom-scroll margin uses the first line’s height instead of the last line’s height.
`CHANGELOG.md`	Documents the feature.

Suppressed comments (4)

src/views/editor/mod.rs:1724

rvline_at_y reads cached_line_count before the cache is invalidated at line 1715. After an edit or config change, it can return a stale wrapped line_index; iter_rvlines_init then clears the cache and treats that index as past the newly unlaid line, so this branch can produce no screen rows. Validate the cache, including config_id, before calling rvline_at_y.

        let start = editor.rvline_at_y(y0);

src/views/editor/mod.rs:779

With this mode enabled, page_move, center_window, top_of_window, bottom_of_window, and scroll still use line_height(0) for document positions and scroll deltas (mod.rs:665-750). A variable-height editor therefore scrolls using a different coordinate system than the rows computed here, leaving the cursor at the wrong vertical position. These commands need accumulated line positions, or the hint must be explicitly limited.

    pub fn per_line_heights_active(&self) -> bool {
        !self.style().uniform_line_height(self.id())
    }

src/views/editor/mod.rs:816

The variable branch recalculates every buffer line on each call, but total_height() is used by both editor layout/measure paths and the gutter measure path. This makes large variable-height documents pay O(document size) repeatedly during layout, rather than the viewport-prefix walk described for this feature; unlike Lines::last_vline, there is no cached result. Cache the total/prefix heights and invalidate them with document, style, and layout-cache changes.

        if self.per_line_heights_active() {
            (0..=self.last_line())
                .map(|line| self.line_height_total(line))
                .sum()

src/views/editor/view.rs:1002

total_height() now accounts for per-line heights, but the scroll-beyond-last-line margin still subtracts the first line's height. When the first line is taller than the last line, the bottom margin is too small to scroll the final row to the top. Use editor.line_height(editor.last_line()) for this margin.

            let last_line_height = editor.total_height();

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@@ -225,7 +225,7 @@ impl EditorGutterView {

                // Height is determined by editor content
                let line_height = f64::from(editor.line_height(0));


+    pub fn per_line_heights_active(&self) -> bool {
+        !self.style().uniform_line_height(self.id())
+    }


                    };

-                    let last_line_height = line_height * (editor.last_vline().get() + 1) as f64;
+                    let last_line_height = editor.total_height();


webergnr added 2 commits August 21, 2026 15:13

fix: position editor lines by real per-line heights, wrapped or not

6f4fcbb

Copilot AI lite review requested due to automatic review settings August 21, 2026 20:37

docs: changelog entry for the variable line height fix

14b253d

Copilot started reviewing on behalf of webergnr August 21, 2026 20:38 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

Navigation Menu

Uh oh!

Position editor lines by their real heights when they vary - #1084

Position editor lines by their real heights when they vary#1084
webergnr wants to merge 3 commits into
lapce:mainfrom
webergnr:variable-line-height

webergnr commented Aug 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -225,7 +225,7 @@ impl EditorGutterView {

		// Height is determined by editor content
		let line_height = f64::from(editor.line_height(0));

Uh oh!

Conversation

webergnr commented Aug 21, 2026

The bug

The fix

Tests

Where it is used

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants